Add scene engine pipeline - #436
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “scene engine” image-to-tabletop-scene generation pipeline under embodichain/gen_sim/scene_engine, covering semantic scene understanding, image segmentation, coarse geometry/layout generation, heuristic layout refinement + gravity settling, Gym export, and CLI preview tooling. It also standardizes use of VHACD for collision decomposition in the simulation-facing parts of the pipeline.
Changes:
- Added core scene data structures (table/assets/scene) plus stage modules for understanding, segmentation, generation/refinement, and Gym export.
- Added service clients/config loaders for an OpenAI-compatible VLM endpoint, an image-segmentation service, and a geometry-generation service.
- Added CLI entry points to run the pipeline and preview the generated Gym export in
SimulationManager.
Reviewed changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| embodichain/gen_sim/scene_engine/utils/logger.py | Adds simple stage logging helpers for the pipeline. |
| embodichain/gen_sim/scene_engine/utils/init.py | Initializes the utils package. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py | Implements RLE mask decode/merge and mask visualization helpers. |
| embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py | Adds geometry/layout utilities: transforms, table support heuristics, packing, and gravity-settling helpers. |
| embodichain/gen_sim/scene_engine/pipeline/utils/init.py | Initializes pipeline utils package. |
| embodichain/gen_sim/scene_engine/pipeline/scene_understanding.py | VLM-driven semantic parsing + strict JSON validation for table/assets. |
| embodichain/gen_sim/scene_engine/pipeline/scene_segmentation.py | Service-driven segmentation with VLM validation/assignment and mask outputs. |
| embodichain/gen_sim/scene_engine/pipeline/scene_generation.py | Orchestrates coarse geometry download, SimReady conversion, refinement, and scene updates. |
| embodichain/gen_sim/scene_engine/pipeline/gym_export.py | Exports final scene + meshes into a Gym config + mesh assets layout. |
| embodichain/gen_sim/scene_engine/pipeline/generate.py | End-to-end pipeline entrypoint tying all stages together. |
| embodichain/gen_sim/scene_engine/pipeline/init.py | Initializes pipeline package. |
| embodichain/gen_sim/scene_engine/llms/openai_compatible_client.py | Implements an OpenAI-compatible multimodal chat-completions client. |
| embodichain/gen_sim/scene_engine/llms/load_config.py | Loads VLM config with environment-variable overrides. |
| embodichain/gen_sim/scene_engine/llms/init.py | Initializes llms package. |
| embodichain/gen_sim/scene_engine/core/table.py | Defines the Table dataclass and serialization. |
| embodichain/gen_sim/scene_engine/core/scene.py | Defines the Scene dataclass and serialization. |
| embodichain/gen_sim/scene_engine/core/asset.py | Defines the Asset dataclass and serialization. |
| embodichain/gen_sim/scene_engine/core/init.py | Initializes core package. |
| embodichain/gen_sim/scene_engine/configs/scene_engine_config.json | Adds a unified config template for VLM/segmentation/geometry services. |
| embodichain/gen_sim/scene_engine/configs/init.py | Initializes configs package. |
| embodichain/gen_sim/scene_engine/clients/image_segmentation.py | Adds image segmentation service client + config loader. |
| embodichain/gen_sim/scene_engine/clients/geometry_generation.py | Adds geometry generation service client + response parsing + downloads. |
| embodichain/gen_sim/scene_engine/clients/init.py | Initializes clients package. |
| embodichain/gen_sim/scene_engine/cli/start.py | Adds CLI entry to run the full pipeline from an image. |
| embodichain/gen_sim/scene_engine/cli/preview.py | Adds CLI tool to preview exported Gym scenes in simulation. |
| embodichain/gen_sim/scene_engine/cli/init.py | Initializes cli package. |
Comments suppressed due to low confidence (1)
embodichain/gen_sim/scene_engine/pipeline/generate.py:106
- If generate_scene_and_refine() (or check_health()) raises, GeometryGenerationClient.close() is skipped, leaking the underlying requests.Session. Use try/finally to ensure cleanup.
# 3. Objects + Coarse Layout Generation
log_stage_start("Objects + Coarse Layout Generation")
# Load the config and fail if the Geometry Generation Server is unavailable.
geometry_generation_client = GeometryGenerationClient.from_config(
geometry_generation_config_path
)
geometry_generation_client.check_health()
scene = generate_scene_and_refine(
image_path=image_path,
output_root=resolved_output_root,
scene=scene,
vlm_client=vlm_client,
geometry_generation_client=geometry_generation_client,
)
geometry_generation_client.close() # Kill the session.
log_stage_end("Objects + Coarse Layout Generation")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 2. Scene Segmentation | ||
| log_stage_start("Scene Segmentation") | ||
| # Load the config and fail if the Image Segmentation Server is unavailable. | ||
| image_segmentation_client = ImageSegmentationClient.from_config( | ||
| image_segmentation_config_path | ||
| ) | ||
| image_segmentation_client.check_health() # Error raising will happen internally. | ||
| scene = segment_scene( | ||
| image_path=image_path, | ||
| output_root=resolved_output_root, | ||
| scene=scene, | ||
| vlm_client=vlm_client, | ||
| image_segmentation_client=image_segmentation_client, | ||
| ) | ||
| image_segmentation_client.close() # Kill the session. | ||
| log_stage_end("Scene Segmentation") |
There was a problem hiding this comment.
I have already fixed this part.
| finally: | ||
| sim.destroy() | ||
|
|
There was a problem hiding this comment.
I have already fixed this part.
| rendered_mask = ( # If weuse outline, then need to do some another processings. | ||
| mask if mask_style == "fill" else _mask_outer_outline(mask, image.size) | ||
| ) |
There was a problem hiding this comment.
I have changed weuse into we use.
| if ( | ||
| scene.table.id != "table" | ||
| ): # Currently it will always return true. For we hardcode the table id to "table". | ||
| raise ValueError("Scene table id must be 'table'.") |
There was a problem hiding this comment.
I have already shorten this into one line.
| def generate_scene_from_image( | ||
| image_path: str | Path, | ||
| output_root: str | Path, | ||
| *, | ||
| llm_config_path: str | Path | None = None, | ||
| image_segmentation_config_path: str | Path | None = None, | ||
| geometry_generation_config_path: str | Path | None = None, | ||
| ) -> Scene: | ||
| """Generate the initial core scene state from an input image.""" |
There was a problem hiding this comment.
Thank you for reviewing!
| print("Successfully completed!") | ||
|
|
||
|
|
||
| def main() -> None: |
There was a problem hiding this comment.
[P2] 请将新增命令注册到统一 CLI。当前项目公开入口是 embodichain console script，但 embodichain.__main__.COMMANDS 中没有 Scene Engine 或预览命令；因此安装后用户无法从统一 CLI 发现或调用这里的 main()，只能依赖内部模块路径。请在 COMMANDS 中注册 Scene Engine 和 preview 对应的子命令。
There was a problem hiding this comment.
Resolved by registering scene-engine and preview-scene in embodichain.__main__.COMMANDS.
The unified dispatcher lazily imports the selected command handler and forwards the remaining command-line arguments to its main(argv) function. Both Scene Engine CLI entry points accept the forwarded argv, allowing their own argument parsers to process the expected options instead of relying on the process-wide sys.argv.
Installed users can invoke Scene Engine through the public CLI:
embodichain scene-engine \
--image <image> \
--output_root <output_root> \
--config <scene_engine_config>
embodichain preview-scene \
--output_root <output_root>preview-scene opens the native preview window by default. To use the browser-based Viser preview:
embodichain preview-scene \
--output_root <output_root> \
--viser \
--viser-host 0.0.0.0 \
--viser-port 9000The corresponding module entry points are:
python -m embodichain.gen_sim.scene_engine.cli.start \
--image <image> \
--output_root <output_root> \
--config <scene_engine_config>
python -m embodichain.gen_sim.scene_engine.cli.preview \
--output_root <output_root>I also verified that:
scene-engineandpreview-sceneappear inembodichain --help;embodichain scene-engine --helpandembodichain preview-scene --helpdispatch to their command-specific parsers correctly;preview-scene --helpexposes the required--output_rootargument and the optional Viser-related flags;- the relevant CLI and Scene Engine unit tests pass (
21 passed).
| resolved_output_root = Path(output_root).expanduser().resolve() | ||
| resolved_output_root.mkdir(parents=True, exist_ok=True) | ||
|
|
||
| generate_scene_from_image( |
There was a problem hiding this comment.
[P1] Pass service configuration through the generation CLI. As written, users cannot run this command without editing the JSON installed with the package: this call forwards only the image and output paths, while the packaged service URLs are empty. The LLM endpoint can be overridden through environment variables, but the segmentation and geometry services have no CLI or environment-variable entry point, so generation fails during base_url validation. Please accept a config path or explicit service URL options here and forward them into the pipeline.
There was a problem hiding this comment.
Resolved by adding an optional --config argument to the generation CLI. The provided Scene Engine JSON configuration is propagated to the LLM, image segmentation, and geometry generation clients, allowing users to override service endpoints without modifying the package-installed default configuration.
| for scene_object in scene_objects | ||
| } | ||
| gym_config = { | ||
| "id": f"Prompt2Scene-{int(time.time() * 1000)}-v0", |
There was a problem hiding this comment.
[P1] Export a registered environment ID. embodichain run-env passes this generated ID directly to gymnasium.make, but the codebase does not register any Prompt2Scene-* environment. Consequently, every exported config fails immediately with NameNotFound. Please export an environment ID that is already registered with the runner, or provide and invoke the corresponding registration logic.
There was a problem hiding this comment.
Resolved by changing the artifact from a Gym environment configuration to an explicit scene export (embodichain.scene-export/v1).
The generated identifier is now scene_id, which uniquely identifies the exported scene data. It is not a Gymnasium environment ID and is never passed to gymnasium.make(). The exported scene is loaded through the Scene Engine preview path instead of embodichain run-env, so no Gym environment registration is required.
| "max_episodes": 10, | ||
| "max_episode_steps": 300, | ||
| "env": {"events": {}, "observations": {}, "dataset": {}}, | ||
| "robot": {}, |
There was a problem hiding this comment.
[P1] Supply a valid robot configuration in the Gym export. Even after the environment ID is fixed, EmbodiedEnv parses this empty object as RobotCfg(fpath=None); _setup_robot then receives None from add_robot and proceeds to access robot joints. If this file is intended to load directly through run-env, export a valid robot configuration. Otherwise, target an explicitly scene-only environment that supports running without a robot.
There was a problem hiding this comment.
Resolved by changing the artifact from a Gym environment configuration to an explicit scene export (embodichain.scene-export/v1).
The generated scene does not infer a robot model, robot placement, controller, or sensor setup from the input image, so exporting a default robot configuration would produce an incomplete and potentially misleading task setup. The scene export therefore omits the robot field entirely and is loaded through the Scene Engine preview path, which directly creates a SimulationManager for the table and assets without constructing an EmbodiedEnv.
A downstream task or environment layer can attach an explicit robot configuration when one is available, keeping scene generation independent from task-specific execution.
| destination_glb_path = mesh_assets_root / object_id / f"{object_id}.glb" | ||
| destination_glb_path.parent.mkdir(parents=True, exist_ok=True) | ||
| shutil.copy2(source_glb_path, destination_glb_path) | ||
| return destination_glb_path.relative_to(mesh_assets_root.parent).as_posix() |
There was a problem hiding this comment.
[P1] Emit mesh paths that the Gym loader can resolve. When this config is loaded through run-env, config_to_cfg passes the relative Mesh path to get_data_path, which interprets mesh_assets as a dataset name and raises Dataset class 'mesh_assets' not found. Only the custom preview resolves this path relative to the config directory. Please export an absolute or data-root-relative path, or update the common loader to resolve Mesh paths relative to the config file.
There was a problem hiding this comment.
Resolved by changing this artifact from a run-env Gym configuration to an explicit scene export (embodichain.scene-export/v1). The output is now written as scene_export/scene_config.json and is loaded through the Scene Engine preview path, which resolves mesh_assets/... relative to the config file directory.
It is no longer passed to config_to_cfg() or get_data_path(), so exporting absolute paths or modifying the common Gym loader is unnecessary here. Keeping config-relative paths also preserves portability when the complete scene_export/ directory is moved.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (13)
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:510
- Calling the private SimulationManager method
_deferred_destroy()bypasses the public teardown path and can become brittle if the internal cleanup mechanism changes. Prefer usingdestroy(exit_process=False)plusSimulationManager.flush_cleanup_queue()to run the deferred cleanup without forcibly exiting the process.
finally:
sim._deferred_destroy()
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:485
RigidObjectCfg.max_convex_hull_num/acd_methodare deprecated in favor of the shape-levelMeshCfgfields. Moving these toMeshCfg(...)avoids relying on deprecated overrides and keeps the config consistent with the rest of the sim stack.
body_scale=tuple(asset_info["y_up_scale"]),
body_type="dynamic",
max_convex_hull_num=max_convex_hull_num,
acd_method="vhacd", # Use vhacd by default.
embodichain/gen_sim/scene_engine/cli/preview.py:89
SimulationManager.destroy()may callos._exit(0)by default (viaEMBODICHAIN_SIM_EXIT_PROCESS), which is surprising for a CLI preview tool. Also, the sim manager uses deferred cleanup; flushing the cleanup queue avoids leaked C++ resources/segfaults on interpreter shutdown (seeembodichain/lab/sim/sim_manager.py:2676-2797).
finally:
sim.destroy()
embodichain/gen_sim/scene_engine/pipeline/generate.py:88
- If
segment_scene(...)raises, theImageSegmentationClientsession never closes. Wrap usage in atry/finallyso the HTTP session is closed on all error paths.
image_segmentation_client = ImageSegmentationClient.from_config(
image_segmentation_config_path
)
image_segmentation_client.check_health() # Error raising will happen internally.
scene = segment_scene(
image_path=image_path,
output_root=resolved_output_root,
scene=scene,
vlm_client=vlm_client,
image_segmentation_client=image_segmentation_client,
)
image_segmentation_client.close() # Kill the session.
log_stage_end("Scene Segmentation")
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:216
- Spelling/grammar in the inline comment: "weuse" / "some another processings".
rendered_mask = ( # If weuse outline, then need to do some another processings.
mask if mask_style == "fill" else _mask_outer_outline(mask, image.size)
embodichain/gen_sim/scene_engine/pipeline/generate.py:54
- This PR introduces a new end-to-end scene-engine pipeline (VLM parsing/validation, RLE decoding, mask unioning, geometry/layout refinement, gym export) but adds no tests. There is existing gen_sim test coverage (e.g.
tests/gen_sim/simready_pipeline/*), so adding unit tests for the pure functions (JSON schema validators, RLE encode/decode, IoU/union logic, layout transform helpers) and lightweight integration tests with mocked clients would help prevent regressions.
def generate_scene_from_image(
image_path: str | Path,
output_root: str | Path,
*,
llm_config_path: str | Path | None = None,
image_segmentation_config_path: str | Path | None = None,
geometry_generation_config_path: str | Path | None = None,
) -> Scene:
embodichain/gen_sim/scene_engine/cli/preview.py:160
RigidObjectCfg.max_convex_hull_num/acd_methodare deprecated (they override the shape-level settings). Set convex-decomposition settings onMeshCfginstead, so preview matches the supported config path.
max_convex_hull_num=max_convex_hull_num,
acd_method="vhacd", # Use vhacd by default.
)
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:468
RigidObjectCfg.max_convex_hull_num/acd_methodare deprecated in favor ofMeshCfg.max_convex_hull_num/MeshCfg.acd_method(seeembodichain/lab/sim/cfg.py:963-984). Set these on theMeshCfginstead to match the supported configuration surface.
This issue also appears on line 482 of the same file.
body_scale=tuple(table_y_up_scale),
body_type="static",
max_convex_hull_num=max_convex_hull_num,
acd_method="vhacd", # Use vhacd by default.
embodichain/gen_sim/scene_engine/cli/preview.py:85
- The preview loop opens a window and then only sleeps; if physics is manually updated (the common case), the viewer may not render / respond because the simulation never steps. Stepping once per loop keeps the window responsive and ensures the scene is actually rendered.
This issue also appears on line 158 of the same file.
sim.open_window()
while True:
time.sleep(0.1)
embodichain/gen_sim/scene_engine/pipeline/generate.py:106
- If
generate_scene_and_refine(...)raises, theGeometryGenerationClientsession is left open. Usetry/finallyto guarantee.close()and avoid leaking connections.
geometry_generation_client = GeometryGenerationClient.from_config(
geometry_generation_config_path
)
geometry_generation_client.check_health()
scene = generate_scene_and_refine(
image_path=image_path,
output_root=resolved_output_root,
scene=scene,
vlm_client=vlm_client,
geometry_generation_client=geometry_generation_client,
)
geometry_generation_client.close() # Kill the session.
log_stage_end("Objects + Coarse Layout Generation")
embodichain/gen_sim/scene_engine/clients/geometry_generation.py:67
check_health()ignores the configuredtimeout_sby hardcodingtimeout=10. If the intention is to cap the timeout for health checks, usemin(self._timeout_s, 10)so the config is still respected when it is smaller.
response = self._session.get(
self._url(self._health_path),
# timeout=self._timeout_s,
timeout=10, # Use a shorter timeout for avoiding long waits.
)
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:188
- Spelling/wording in comments: "assets(includes table)", "seperately".
# Simready all the assets(includes table).
# Treat table and assets seperately.
# Notice that, currently the simready process is only
embodichain/gen_sim/scene_engine/utils/logger.py:22
- Logger naming is inconsistent with the rest of the codebase, which generally uses
logging.getLogger(__name__)(e.g.embodichain/utils/logger.py:24,embodichain/lab/gym/utils/registration.py:41). Using__name__keeps the logger hierarchy aligned with the module path and makes filtering/configuration easier.
_LOGGER = logging.getLogger("embodichain.scene_engine")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (8)
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:479
- Same as the table config above:
RigidObjectCfg.max_convex_hull_num/acd_methodare deprecated; for new code setmax_convex_hull_num/acd_methodon theMeshCfgshape so collision decomposition settings live with the mesh (and avoid legacy overrides).
RigidObjectCfg(
uid=asset_id,
shape=MeshCfg(fpath=str(asset_info["mesh_path"])),
init_pos=tuple(rigid_layout["pos"]),
init_rot=tuple(
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:511
- Calling the private
SimulationManager._deferred_destroy()couples this pipeline to internal cleanup implementation details and bypasses the supporteddestroy()API. Sincedestroy()can already avoidos._exit()viaexit_process=False, prefer the public method and then flush the cleanup queue for deterministic teardown inside the pipeline.
finally:
sim._deferred_destroy()
embodichain/gen_sim/scene_engine/pipeline/scene_understanding.py:180
- The inline comment is incorrect/misleading: since the table id is hardcoded to "table" in
validate_scene_understanding_json, this condition will normally be false (it won't "always return true"). Consider rewording so readers don't misinterpret the control flow.
if (
scene.table.id != "table"
): # Currently it will always return true. For we hardcode the table id to "table".
raise ValueError("Scene table id must be 'table'.")
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:216
- Spelling/grammar in the comment makes it harder to read ("weuse", "processings").
rendered_mask = ( # If weuse outline, then need to do some another processings.
mask if mask_style == "fill" else _mask_outer_outline(mask, image.size)
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:469
RigidObjectCfg.max_convex_hull_num/acd_methodare marked deprecated inembodichain.lab.sim.cfg.RigidObjectCfg(preferMeshCfg.max_convex_hull_num/MeshCfg.acd_method). Since this is new code, set these onMeshCfginstead of the top-level cfg to match the current API and avoid relying on legacy overrides.
This issue also appears on line 475 of the same file.
RigidObjectCfg(
uid=table_id,
shape=MeshCfg(fpath=str(table_mesh_path)),
init_pos=tuple(table_rigid_layout["pos"]),
init_rot=tuple(
embodichain/gen_sim/scene_engine/cli/preview.py:160
RigidObjectCfg.max_convex_hull_num/acd_methodare deprecated in favor ofMeshCfg.max_convex_hull_num/MeshCfg.acd_method(seeembodichain.lab.sim.cfg.RigidObjectCfg). For new code, attach VHACD settings to theMeshCfgso they stay with the shape config.
RigidObjectCfg(
uid=uid,
shape=MeshCfg(fpath=str(mesh_path)),
# Keep every preview body static: exported poses are already the
# final gravity-settled poses and should not be simulated again.
embodichain/gen_sim/scene_engine/pipeline/gym_export.py:189
- PR description says VHACD is the default collision decomposition method, but the exported gym config does not specify any
acd_method. In EmbodiChain,MeshCfg.acd_methoddefaults to"coacd"(seeembodichain/lab/sim/shapes.py), so consumers loading this gym config viaRigidObjectCfg.from_dict()will still decompose with CoACD unless explicitly overridden.
"shape": {
"shape_type": "Mesh",
"fpath": asset_relative_path,
"compute_uv": False,
},
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:55
- This PR introduces substantial new parsing/validation logic (e.g., RLE decoding, IoU-based mask merging, strict VLM JSON parsing) but does not add unit tests. The repo already has
tests/gen_sim/coverage for similar geometry/pipeline utilities, so adding focused tests would help prevent regressions (e.g., RLE decode/encode round-trip, union_overlapping_mask_candidates grouping, and error handling for malformed payloads).
def decode_rle_mask(mask_rle: dict[str, Any]) -> Image.Image:
"""Decode an uncompressed RLE mask into a binary image."""
# Check the return value's format.
size = mask_rle.get("size")
counts = mask_rle.get("counts")
if (
not isinstance(size, list)
or len(size) != 2
or not all(isinstance(value, int) and value > 0 for value in size)
):
raise ValueError("Image Segmentation Server RLE needs size=[height, width].")
if not isinstance(counts, list):
raise ValueError("Image Segmentation Server RLE counts must be a list.")
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (9)
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:215
- Typo/grammar in the inline comment ("weuse" / "processings"). This comment is user-facing for future maintainers; please correct it for clarity.
rendered_mask = ( # If weuse outline, then need to do some another processings.
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:195
Image.open(...).convert(...)leaves the file handle open until the Image is closed. Wrap the open in a context manager so repeated calls don't leak descriptors.
image = Image.open(image_path).convert("RGBA")
embodichain/gen_sim/scene_engine/pipeline/utils/scene_generation_utils.py:510
- Calling SimulationManager's private
_deferred_destroy()ties this pipeline to an internal cleanup implementation. Prefer using the publicdestroy(exit_process=False)plusSimulationManager.flush_cleanup_queue()to guarantee cleanup without relying on a private method.
finally:
sim._deferred_destroy()
embodichain/gen_sim/scene_engine/pipeline/generate.py:80
image_segmentation_client.close()is not in afinallyblock, so an exception during segmentation/validation will leak the underlyingrequests.Session. Wrap the segmentation call intry/finallyto always close the client.
image_segmentation_client = ImageSegmentationClient.from_config(
image_segmentation_config_path
)
image_segmentation_client.check_health() # Error raising will happen internally.
scene = segment_scene(
embodichain/gen_sim/scene_engine/pipeline/scene_understanding.py:180
- The inline comment says the condition "will always return true", but the code hardcodes
id="table", so the condition should never be true. Updating the comment avoids misleading future debugging.
if (
scene.table.id != "table"
): # Currently it will always return true. For we hardcode the table id to "table".
raise ValueError("Scene table id must be 'table'.")
embodichain/gen_sim/scene_engine/pipeline/generate.py:54
- This PR adds a large new scene-engine pipeline (VLM JSON validation, RLE decoding, layout transforms, export schema) but does not add any tests. The repo already has
tests/gen_sim/simready_pipeline/*, so adding focused unit tests (e.g., RLE decode/encode round-trip, transform_matrix<->layout_object invariants, exported scene_config schema) would prevent regressions.
def generate_scene_from_image(
image_path: str | Path,
output_root: str | Path,
*,
llm_config_path: str | Path | None = None,
image_segmentation_config_path: str | Path | None = None,
geometry_generation_config_path: str | Path | None = None,
) -> Scene:
embodichain/gen_sim/scene_engine/pipeline/utils/scene_segmentation_utils.py:170
- Image files are opened without a context manager here (and in the mask loop). PIL keeps the underlying file handle open until the Image is closed, which can exhaust file descriptors in batch runs. Use
with Image.open(...)and keep only the converted in-memory image.
This issue also appears on line 195 of the same file.
image = Image.open(image_path).convert("RGB")
ignored_mask = Image.new("L", image.size, 0)
for mask_path in mask_paths:
mask = Image.open(mask_path).convert("L")
_require_image_size(mask, image.size)
embodichain/gen_sim/scene_engine/pipeline/generate.py:106
geometry_generation_client.close()is not protected byfinally, so failures during geometry generation/refinement can leak therequests.Session. Usetry/finallyaround the client usage to ensure it is always closed.
geometry_generation_client = GeometryGenerationClient.from_config(
geometry_generation_config_path
)
geometry_generation_client.check_health()
scene = generate_scene_and_refine(
embodichain/gen_sim/scene_engine/cli/preview.py:90
- In interactive mode the preview loop never advances the simulation (
sim.update(...)). If physics is manually stepped (the common case in SimulationManager), the window may never render/refresh. Stepping at a low rate keeps the viewer responsive and consistent with the headless path.
sim.open_window()
while True:
time.sleep(0.1)
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
Suppressed comments (4)
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:368
- _extract_largest_support_polygon() reconstructs a new Polygon from only the exterior ring (Polygon(boundary_xy)), which drops any interior holes in the detected support region. That can allow downstream clamping/optimization to place assets over holes/cutouts that were present in the merged_region result.
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:32 - Calling matplotlib.use("Agg") at import time can raise if matplotlib.pyplot was already imported elsewhere in the process (Matplotlib disallows backend switching after pyplot import). Using force=True avoids import-time failures in such environments while still enforcing a non-interactive backend for debug rendering.
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:26 - Calling matplotlib.use("Agg") at import time can raise if matplotlib.pyplot was already imported elsewhere in the process. Consider using force=True to prevent import-time failures while still selecting a headless backend for debug images.
matplotlib.use("Agg")
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_layout_optimizer.py:25
- Calling matplotlib.use("Agg") at import time can raise if matplotlib.pyplot was already imported elsewhere in the process. Consider using force=True to prevent import-time failures while still selecting a headless backend for debug images.
matplotlib.use("Agg")
|
Next you should think about how to add the articulation into the scene engine. A possible solution is to add a special signal to use the articraft-style articulation pipeline. Then put it into the scene by using 2d optimization. Whether generating articulation depends on whether we want to interact with this object. |
MuziWong
added
docs
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
Suppressed comments (8)
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:99
- Spelling: "seperately" is misspelled in this comment.
# Treat table and assets seperately.
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:200
- Spelling: "cacheing" should be "caching" in this comment.
# Compute feasible-centre maps with cacheing to avoid repeated binary erosion
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:98
- Spelling/capitalization: "Simready" should match the project's "SimReady" terminology, and this sentence also misses a space before the parenthesis.
This issue also appears on line 99 of the same file.
# Simready all the assets(includes table).
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_layout_optimizer.py:168
- Spelling: "handeled" should be "handled" in this comment.
# If the overlaps is handeled by a previous pair, skip it.
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:197
- Spelling/grammar: "AABB.s" should be "AABBs" in this comment.
This issue also appears on line 200 of the same file.
# Rasterize the support region and compute feasible-centre maps for each AABB.s
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_table_aligner.py:143
- _convert_layout_coordinate_system() indexes layout_object["id"], which raises a KeyError (with a less actionable stack trace) if callers pass an invalid layout object. Prefer reusing _require_layout_id() so invalid inputs raise a consistent ValueError.
return transform_matrix_to_layout_object(
str(layout_object["id"]),
source_to_target_matrix
@ layout_object_to_transform_matrix(layout_object)
@ np.linalg.inv(source_to_target_matrix),
embodichain/gen_sim/scene_engine/pipeline/utils/assets_gravity_settler.py:329
- _convert_layout_coordinate_system() indexes layout_object["id"], which raises KeyError for invalid input. Use _require_layout_id() for consistent validation/error messaging.
return transform_matrix_to_layout_object(
str(layout_object["id"]),
source_to_target_matrix
@ layout_object_to_transform_matrix(layout_object)
@ np.linalg.inv(source_to_target_matrix),
)
docs/source/features/generative_sim/scene_engine.md:11
- This page instructs users to install the
gensimextra, but this PR also adds a more targetedscene-engineoptional dependency group in pyproject.toml. Consider mentioningscene-enginehere (and optionally note thatgensimalso includes it) so users don't have to pull Blender-related deps unnecessarily.
Install EmbodiChain with the `gensim` extra first; see
[Installation](../../quick_start/install.md#optional-generative-simulation-gensim).
…mready pipeline ones
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Suppressed comments (1)
embodichain/gen_sim/scene_engine/cli/preview.py:112
preview_scene_export()opens the native window / Viser and then sleeps forever without stepping the simulation. This diverges from other preview tooling (e.g.embodichain/lab/scripts/preview_asset.py) and can leave the window/Viser session unresponsive or never publishing frames. Step the simulation in the loop (even with static bodies) to keep the preview alive.
sim.open_window()
print("Close with Ctrl-C.")
while True:
time.sleep(0.1)
| scene-engine = [ | ||
| "requests", | ||
| "Pillow", | ||
| "numpy", | ||
| "scipy", | ||
| "shapely", | ||
| "trimesh", | ||
| "open3d", | ||
| "matplotlib" | ||
| ] | ||
| gensim = [ | ||
| "bpy", | ||
| "pyrender==0.1.45" | ||
| "pyrender==0.1.45", | ||
| "requests", | ||
| "Pillow", | ||
| "numpy", | ||
| "scipy", | ||
| "shapely", | ||
| "trimesh", | ||
| "open3d", | ||
| "matplotlib" | ||
| ] |
There was a problem hiding this comment.
Thanks — addressed.
Scene Engine and SimReady Pipeline are peer components of the gen_sim module, so we kept gensim as their shared installation extra rather than adding a separate scene-engine extra.
Changes
- Kept
gensimas the shared installation extra for both Scene Engine and SimReady Pipeline. - Explicitly added Scene Engine's direct runtime dependencies to the
gensimextra:requestsPillownumpyscipyshapelytrimeshopen3dmatplotlib
- Fixed the missing TOML commas.
- Added a CI import/install check for the
gensiminstallation path.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 42 out of 42 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:124
- The layout-refinement result is assigned to local variables but never used. This makes it look like these values matter when the function relies on _layout_refinement mutating the Scene instead. Either use the returned layouts, or call _layout_refinement without capturing the result.
refined_table_layout, refined_assets_layout = _layout_refinement(
scene=scene, # Update this data structure internally.
simready_geometry_output_root=simready_geometry_output_root, # Contains simready assets and their current coarse layout JSON.
debug_output_root=debug_output_root, # Keep the table support surface info + optimized layout info (render with matplotlib) for debugging.
)
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:100
- Spelling/grammar in these comments: "Simready" should be "SimReady", and "seperately" should be "separately". Keeping these stage comments clean helps when scanning pipeline logs and debugging output artifacts.
# Simready all the assets(includes table).
# Treat table and assets seperately.
coarse_layout = _load_layout(coarse_geometry_output_root / "coarse_layout.json")
| object_id = scene_object.id | ||
| if Path(object_id).name != object_id or object_id in {"", ".", ".."}: | ||
| raise ValueError( | ||
| f"Scene object id is not safe for a GLB filename: {object_id!r}" |
There was a problem hiding this comment.
- Addressed.
SceneExporternow rejects backslashes inobject_id, matchingGeometryGenerationClient's filename-safety checks. This prevents Windows path separators from bypassing validation and escaping the intended output directory. - Added a regression test for malicious IDs such as
..\\evil.
…fied scene export
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated no new comments.
Suppressed comments (6)
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:200
- Typo in comment: "cacheing" should be "caching".
# Compute feasible-centre maps with cacheing to avoid repeated binary erosion
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:197
- Typo in comment: "AABB.s" should be "AABBs".
This issue also appears on line 200 of the same file.
# Rasterize the support region and compute feasible-centre maps for each AABB.s
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:478
- Support-geometry repair can return a GeometryCollection that contains MultiPolygon members (not just Polygon). The current code drops those MultiPolygon pieces, which can incorrectly shrink/alter the usable support region and make otherwise-feasible placements fail.
if isinstance(repaired, GeometryCollection):
polygons = [item for item in repaired.geoms if isinstance(item, Polygon)]
return MultiPolygon(polygons) if polygons else None
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:352
- When unary_union returns a GeometryCollection, it can contain MultiPolygon members. The current extraction keeps only geometries with geom_type == "Polygon" at the top level, which can drop polygons nested in MultiPolygons and reduce the detected support contour incorrectly.
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_layout_optimizer.py:168 - Typo in comment: "handeled" should be "handled".
# If the overlaps is handeled by a previous pair, skip it.
embodichain/gen_sim/scene_engine/pipeline/scene_generation.py:99
- Typo in comment: "seperately" should be "separately".
# Treat table and assets seperately.
| "pyrender==0.1.45", | ||
| "requests", | ||
| "Pillow", | ||
| "numpy", |
There was a problem hiding this comment.
numpy, shapely, trimesh and open3d already included in dexsim. So please remove them
There was a problem hiding this comment.
Updated pyproject.toml as requested.
Changes
- Removed the following dependencies from the
gensimextra because they are already provided bydexsim_engine:numpyshapelytrimeshopen3d
- Kept the remaining Scene Engine dependencies explicitly declared in the
gensimextra. - Verified that the optional dependency list remains valid TOML.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Suppressed comments (4)
embodichain/gen_sim/scene_engine/pipeline/utils/table_support_surface.py:35
- Calling
matplotlib.use("Agg")at module import time can crash ifmatplotlib.pyplotwas imported earlier in the process, and it also globally overrides the backend for any other code. Guard this call so it only runs when pyplot hasn't been imported yet.
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_support_clamp.py:28 matplotlib.use("Agg")runs at import time here, which can raise if pyplot is already imported and also globally changes the backend for unrelated code. Add asys.modulesguard so the backend is only set when safe.
import matplotlib
matplotlib.use("Agg")
import matplotlib.pyplot as plt
embodichain/gen_sim/scene_engine/pipeline/utils/assets_group_layout_optimizer.py:27
matplotlib.use("Agg")is executed during import, which can throw if pyplot was imported earlier and forces a global backend change. Guard it (same pattern as other headless-safe modules) so the import is robust in interactive contexts.
import matplotlib
matplotlib.use("Agg")
import matplotlib.pyplot as plt
embodichain/gen_sim/scene_engine/cli/preview.py:112
- In the interactive preview path, the code opens a native window but never calls
sim.update().SimulationManager.open_window()only opens the window; without stepping, meshes/lights may not render and the viewer can appear frozen. Callsim.update(step=1)at least once after opening the window, and keep stepping in the preview loop (static bodies will remain static).
if is_viser:
sim.update(step=1)
print(f"Previewing in Viser: {config_path}")
else:
print(f"Previewing: {config_path}")
sim.open_window()
print("Close with Ctrl-C.")
while True:
time.sleep(0.1)
| [project.optional-dependencies] | ||
| gensim = [ | ||
| "bpy", | ||
| "pyrender==0.1.45" | ||
| "pyrender==0.1.45", | ||
| "requests", | ||
| "Pillow", | ||
| "scipy", | ||
| "matplotlib", | ||
| ] |
Description
This PR adds the image to tabletop scene generation pipeline, including semantic understanding, image segmentation and verification, geometry generation, layout refinement, gravity settling, Gym export, and preview tooling.
Dependencies: Existing scene-engine service endpoints and simulation dependencies.
Type of change
Screenshots
N/A
Checklist
black .command to format the code base. (blackis unavailable in the currentenvironment; the branch includes its existing
RAN blackcommit.)